show-targets #36 by alphastrata · Pull Request #79 · Rust-GPU/cargo-gpu

alphastrata · 2025-06-02T00:23:42Z

Takes a stab at #36, not sure of the methodology for retrieving the targets, but the plumbing is here for when we decide the best method.

$ cargo gpu show targets --shader-crate ../shader-crate-template
All available targets for rust-gpu version 'https://github.com/Rust-GPU/rust-gpu+86fc4803':
spirv-unknown-opengl4.0
spirv-unknown-opengl4.1
spirv-unknown-opengl4.2
spirv-unknown-opengl4.3
spirv-unknown-opengl4.5
spirv-unknown-spv1.0
spirv-unknown-spv1.1
spirv-unknown-spv1.2
spirv-unknown-spv1.3
spirv-unknown-spv1.4
spirv-unknown-spv1.5
spirv-unknown-vulkan1.0
spirv-unknown-vulkan1.1
spirv-unknown-vulkan1.1spv1.4
spirv-unknown-vulkan1.2

closes #36

comments

Firestar99

I would suggest you wait with this one a little until #75 is merged, as it changes how we forward "target specs" to our codegen backend. Specifically, we'll copy the "target specs" from the new rustc_codegen_spirv-target-specs crate to ~/.cache/rust-gpu/codegen/<version_string>/target-specs/, so you should just be able to list all files in that directory. But note that some targets, like opencl, will just error as "unsupported" if you try to use them.

I've also just proposed to make an enum Target of all targets, to give us compile-time error reporting on the target name. Waiting for suggestions on that one.

Relevant code:
af15805#diff-c1ca29f6b5cc2db8f474ffc26467038108c480552a0961c3442ec6ca81efa18bR195

Firestar99 · 2025-06-02T09:52:49Z

crates/cargo-gpu/src/show.rs

+        "spirv-unknown-vulkan1.2",
+    ];
+
+    let output = Command::new("spirv-val")


We cannot guarantee that the end user has spirv-val installed on their system. Cargo gpu compiles spirv-tools itself when building rustc_codegen_spirv and statically links it, and in theory you'd have to ask that one.

Yep -- had feeling there'd be a better way. TY

We can just use pub use include_str::TARGET_SPECS; no?

~/.cache/rust-gpu/codegen/<version_string>/target-specs/'s dir contents would be more tedious & obviously non friendly to those unfortunate enough to have to develop on Windows.

It wouldn't be too hard (I think?) to whip up a const fn that just lists of the ones that were used to compile cargo gpu itself, but it seems a shame to support only specs that were compiled in during install... no?

Note where that symbol is coming from:

cargo-gpu/crates/cargo-gpu/src/legacy_target_specs.rs

Lines 1 to 4 in b497234

//! Legacy target specs are spec jsons for versions before `rustc_codegen_spirv-target-specs`

//! came bundled with them. Instead, cargo gpu needs to bundle these legacy spec files. Luckily,

//! they are the same for all versions, as bundling target specs with the codegen backend was

//! introduced before the first target spec update.

If future versions of rust-gpu introduce new targets (such as spirv-unknown-vulkan1.3), they won't be listed. Which also means targets are dependent on the version of rust-gpu used. I'd recommend doing the same as cargo build does, have a --shader-crate arg that defaults to ./.

alphastrata · 2025-06-03T13:31:54Z

The TARGET_SPECS const has them from compile time, so we should have no issues doing that.

I think we want to avoid doing ~/.cache/rust-gpu/codegen/<version_string>/target-specs/ and just reading the dir's contents, that's not very platform agnostic.

Firestar99 · 2025-06-03T16:10:49Z

There are 4 version ranges of rustc_codegen_spirv and they all need different handling on their target specs:

"ancient" versions such as 0.9.0 or earlier do not need target specs, just passing the target string (spirv-unknown-vulkan1.2) directly is sufficient. We still prep target-specs for them like the "legacy" variant below, spirv-builder will just ignore it
"legacy" versions require target specs to compile, which is a requirement introduced by some rustc version. Back then it was decided that cargo gpu would ship them, as they'd probably never change, right? So now we're stuck with having to ship these "legacy" target specs with cargo gpu forever. These are the symbol legacy_target_specs::TARGET_SPECS, with legacy_target_specs being a fixed version of rustc_codegen_spirv-target-specs, which must never update.
As of PR 256 rustc_codegen_spirv now has a direct dependency on rustc_codegen_spirv-target-specs, allowing cargo gpu to pull the required target specs directly from that dependency. At this point, the target specs are still the same as the legacy target specs.
The upcoming edition 2024 PR must update the target specs to comply with newly added validation within rustc. This is why the new system was implemented, so we can support both old and new target specs without having to worry which version of cargo gpu you are using. It'll "just work".

So while your submitted version will work for now, if I add support for the spirv-unknown-vulkan1.3 and 1.4 targets next week, your tool will not display them. Cause you're relying on the legacy_target_specs::TARGET_SPECS symbol, which will never be updated to newer versions to keep supporting the "legacy" versions. You have no other choice than listing the files in that directory.

alphastrata · 2025-06-04T03:09:31Z

There are 4 version ranges of rustc_codegen_spirv and they all need different handling on their target specs:

* "ancient" versions such as 0.9.0 or earlier do not need target specs, just passing the target string (`spirv-unknown-vulkan1.2`) directly is sufficient. We still prep target-specs for them like the "legacy" variant below, spirv-builder [will just ignore it](https://github.com/Rust-GPU/rust-gpu/blob/369122e1703c0c32d3d46f46fa11ccf12667af03/crates/spirv-builder/src/lib.rs#L987)

* "legacy" versions require target specs to compile, which is a requirement introduced by some rustc version. Back then it was decided that cargo gpu would ship them, as they'd probably never change, right? So now we're stuck with having to ship these "legacy" target specs with cargo gpu _forever_. These are the symbol `legacy_target_specs::TARGET_SPECS`, with `legacy_target_specs` being a **fixed** version of `rustc_codegen_spirv-target-specs`, which must **never** update.

* As of [PR 256](https://github.com/Rust-GPU/rust-gpu/pull/256) `rustc_codegen_spirv` now has a direct dependency on `rustc_codegen_spirv-target-specs`, allowing cargo gpu to pull the required target specs directly from that dependency. At this point, the target specs are still the same as the legacy target specs.

* The upcoming [edition 2024 PR](https://github.com/Rust-GPU/rust-gpu/pull/249) must update the target specs to comply with newly added validation within rustc. This is why the new system was implemented, so we can support both old and new target specs without having to worry which version of cargo gpu you are using. It'll "just work".

So while your submitted version will work for now, if I add support for the spirv-unknown-vulkan1.3 and 1.4 targets next week, your tool will not display them. Cause you're relying on the legacy_target_specs::TARGET_SPECS symbol, which will never be updated to newer versions to keep supporting the "legacy" versions. You have no other choice than listing the files in that directory.

Ok, this directory ~/.cache/rust-gpu does NOT exist when we install via the README instructions though on Windows or MacOS, which is what I (admittedly poorly in hindsight) was trying to point out here: #79 (comment).

On Windows: It should be at %AppData%\Local\rust-gpu, but currently we're not making a directory there (the create_dir_all call is failing silently).

On Mac: It should be at ~/Library/Applications\ Support/rust-gpu, which again, we're currently not making (the create_dir_all call is again failing silently).

There is nothing unusual about my particular setups' dir structures or permissions on either of these machines, I'm not familiar enough with what cargo install does under the hood to explain why.
The logging in the build script's never being initialised so $env:RUST_LOG="trace"; cargo install --path . etc is currently not of any use in diagnosing why, for whatever reason cargo seems to eat everything (even if you add and initialise a logger).

If we can get the cache-dir actually created (for example, if we have the build.rs create it like so, then the implementation you want will work, I've verified this works with Windows 11, not yet with Mac.

It is because of all this, that I went for the contents of the TARGET_SPECS const (for now).

So, do we want to park this until the edition 2024 PR goes in?

I'm happy to update it/make changes etc.

use the `cache_dir`, but fallback on the legacy const as it's known to be there, currently the cache_dir's presence is not guaranteed.

Firestar99 · 2025-06-04T07:39:40Z

cargo install just builds the binary in release mode and places it into your $path, it does not run the executable or create any folder for it. When you run your first install, we create the cache dir:

cargo-gpu/crates/cargo-gpu/src/install.rs

Lines 283 to 288 in b497234

    
           // Ensure the cache dir exists 
        
           let cache_dir = cache_dir()?; 
        
           log::info!("cache directory is '{}'", cache_dir.display()); 
        
           std::fs::create_dir_all(&cache_dir).with_context(|| { 
        
               format!("could not create cache directory '{}'", cache_dir.display()) 
        
           })?;

And create ~/.cache/rust-gpu/codegen/<version>/src (which makes creating the cache dir kinda redundant)

cargo-gpu/crates/cargo-gpu/src/install.rs

Lines 157 to 160 in b497234

    
           log::trace!("writing dummy lib.rs"); 
        
           let src = checkout.join("src"); 
        
           std::fs::create_dir_all(&src).context("creating 'src' directory")?; 
        
           std::fs::File::create(src.join("lib.rs")).context("creating 'src/lib.rs'")?;

And create the ~/.cache/rust-gpu/codegen/<version>/target-specs dir and fill it with the correct content:

cargo-gpu/crates/cargo-gpu/src/install.rs

Line 214 in b497234

fn update_spec_files(

Firestar99 · 2025-06-04T07:51:42Z

Mind if I give this a try?

alphastrata · 2025-06-04T08:15:46Z

Mind if I give this a try?

Of course not :)

…hout updating

Firestar99 · 2025-06-04T08:43:15Z

I've noticed I've added some special handling around local file paths to not have to copy the target specs, which required some refactoring to also make it work with that command.

Now it prints this if successful, and may fail if the version is not installed or the shader crate's path is not a proper shader crate.

All available targets for rust-gpu version 'https://github.com/Rust-GPU/rust-gpu+86fc4803':
spirv-unknown-opengl4.0
spirv-unknown-opengl4.1
spirv-unknown-opengl4.2
spirv-unknown-opengl4.3
spirv-unknown-opengl4.5
spirv-unknown-spv1.0
spirv-unknown-spv1.1
spirv-unknown-spv1.2
spirv-unknown-spv1.3
spirv-unknown-spv1.4
spirv-unknown-spv1.5
spirv-unknown-vulkan1.0
spirv-unknown-vulkan1.1
spirv-unknown-vulkan1.1spv1.4
spirv-unknown-vulkan1.2

alphastrata · 2025-06-04T09:03:14Z

Delicious.

alphastrata · 2025-06-04T09:59:05Z

It looks like I completely missed install.rs' functionality is rather that of working with targets, not something that is part of the install process... whoops.

Firestar99 · 2025-06-04T15:28:33Z

I've now changed this PR significantly to the point where I'd prefer if @tombh or @schell approve it instead :D

tombh · 2025-06-05T12:38:28Z

I think this is great! Especially the doc comments.

I'd be happy to approve this, but I know schell has worked with the target specs more than me, so maybe wait a bit to see if he has any comments?

Firestar99 · 2025-06-06T15:56:05Z

@tombh I haven't seen schell review anything the last couple weeks, probably busy with irl stuff*. Given that and me having refactored the system in #75 I'd rather you approve and we merge this now, than wait for schell to reappear.

schell · 2025-08-02T18:13:36Z

This all seems fine :)

You are right, I've moved house and switched jobs so lots of IRL stuff going on ATM 🙇

show-targets

848a198

comments

Firestar99 requested changes Jun 2, 2025

View reviewed changes

alphastrata added 2 commits June 3, 2025 16:38

Merge branch 'main' into show_targets

5570a22

TARGET_SPECS has them all at comp time

97b56dd

alphastrata force-pushed the show_targets branch from 66e3669 to 97b56dd Compare June 3, 2025 13:28

alphastrata marked this pull request as ready for review June 3, 2025 13:29

alphastrata added 6 commits June 4, 2025 13:30

Update methodology of retrieving targets.

a617029

use the `cache_dir`, but fallback on the legacy const as it's known to be there, currently the cache_dir's presence is not guaranteed.

filter earlier on to do less hashing.

b4ca3e8

check dir exists earlier on.

243b1dd

use same error conventions as already in codebase (anyhow)

c96b613

docs, cleanups

39774f1

chore: silence clippy

b438d7d

create mod target_specs and move all related code there

1b25b5a

Firestar99 added 2 commits June 4, 2025 10:39

refactor update_spec_files to allow querying target_specs dir wit…

ccd2e25

…hout updating

make show targets use new target_specs path querying

76aa6af

fix clippy

37e32c3

update docs on target specs

042da7e

Firestar99 mentioned this pull request Jun 5, 2025

Various small fixes for Graphite #80

Merged

tombh approved these changes Jun 6, 2025

View reviewed changes

Firestar99 approved these changes Jun 6, 2025

View reviewed changes

Firestar99 merged commit 91ade61 into Rust-GPU:main Jun 6, 2025
6 checks passed

	//! Legacy target specs are spec jsons for versions before `rustc_codegen_spirv-target-specs`
	//! came bundled with them. Instead, cargo gpu needs to bundle these legacy spec files. Luckily,
	//! they are the same for all versions, as bundling target specs with the codegen backend was
	//! introduced before the first target spec update.

Conversation

alphastrata commented Jun 2, 2025 • edited by Firestar99 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Firestar99 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Firestar99 Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alphastrata Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

alphastrata Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

Firestar99 Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

alphastrata commented Jun 3, 2025

Uh oh!

Firestar99 commented Jun 3, 2025

Uh oh!

alphastrata commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Firestar99 commented Jun 4, 2025

Uh oh!

Firestar99 commented Jun 4, 2025

Uh oh!

alphastrata commented Jun 4, 2025

Uh oh!

Firestar99 commented Jun 4, 2025

Uh oh!

alphastrata commented Jun 4, 2025

Uh oh!

alphastrata commented Jun 4, 2025

Uh oh!

Firestar99 commented Jun 4, 2025

Uh oh!

tombh commented Jun 5, 2025

Uh oh!

Firestar99 commented Jun 6, 2025

Uh oh!

Uh oh!

schell commented Aug 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alphastrata commented Jun 2, 2025 •

edited by Firestar99

Loading

Firestar99 left a comment •

edited

Loading

Firestar99 Jun 2, 2025 •

edited

Loading

alphastrata commented Jun 4, 2025 •

edited

Loading